AOPP support #1903
AOPP support #1903
Conversation
common/protob/messages-bitcoin.proto
Outdated
| @@ -152,6 +152,7 @@ message SignMessage { | |||
| required bytes message = 2; // message to be signed | |||
| optional string coin_name = 3 [default='Bitcoin']; // coin to use for signing | |||
| optional InputScriptType script_type = 4 [default=SPENDADDRESS]; // used to distinguish between various address formats (non-segwit, segwit, etc.) | |||
| optional bool no_script_type = 5 [default=false]; // don't include script type information in the recovery byte of the signature, same as in Bitcoin Core | |||
There was a problem hiding this comment.
Isn't [default=false] redundant here?
There was a problem hiding this comment.
Removed ed627c4.
There was a problem hiding this comment.
Overall this looks very good, however there seems to be a problem with the legacy pagination because it shows garbage at the end of TestMsgVerifymessage::test_message_long:
The PR changes some ButtonRequests from ProtectCall to Other, we might want to notify Suite of this change. TT currently always uses Other.
UI diff ack.
There was a bug in
Yes, I wanted to unify the T1/TT behavior. Do we have some agreed-upon way of communicating such changes? Just posting it on slack? |
- Add "Confirm message" subheader to SignMessage and VerifyMessage. - Unify font for SignMessage and VerifyMessage to fixed-width font. - Fix device tests.
… not null-terminated.
andrewkozlik
force-pushed
the
andrewkozlik/aopp
branch
from
38a43ac to
0fe6343
Compare
| br_type = "sign_message" | ||
|
|
||
| text = Text(header, new_lines=False) |
There was a problem hiding this comment.
this should be using confirm_address() layout
There was a problem hiding this comment.
...i wanted to make an issue to fix consistency of this, but it's probably not worth it while we expect to move to Rust UI.
Implements https://github.com/trezor/trezor-firmware/issues/1586.
boolparameter toSignMessagein T1 and TT calledno_script_type, which isfalseby default and when set totrueSignMessagedoes not include script type information in the recovery byte of the signature, same as in Bitcoin Core.SignMessageon T1 so that up to 1024 characters can be displayed.SignMessageandEthereumSignMessageon T1 and TT.SignMessageandVerifyMessage.SignMessageandVerifyMessageto monospaced. (SignMessageused to show the message in normal font andVerifyMessagein monospaced.)I didn't tweak the message color as originally proposed in https://github.com/trezor/trezor-firmware/issues/1586#issuecomment-848653922, because after discussion with @matejzak we concluded that users probably wouldn't understand the color distinction. Nevertheless, I already added a parameter to
paginate_text()so that changing the text color will be a one-line change if we choose to do this in the future.